Skip to content

feat(crypto): Add support for encrypted state events to matrix-sdk-crypto #5539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 19, 2025

Conversation

kaylendog
Copy link
Contributor

@kaylendog kaylendog commented Aug 15, 2025

Depends on #5511, #5512 and #5523.

  • Add OutboundGroupSession::encrypt_state.
  • Add GroupSessionManager::encrypt_state and a private helper function ::encrypt_inner.
  • Add OlmMachine::encrypt_state_event and ::encrypt_state_event_raw.
  • Add naive state key unpacking and verification to OlmMachine::decrypt_room_event_inner.

@kaylendog kaylendog self-assigned this Aug 15, 2025
Copy link

codspeed-hq bot commented Aug 15, 2025

CodSpeed Performance Report

Merging #5539 will not alter performance

Comparing kaylendog/msc3414/crypto (13ee4c8) with main (7bbd02c)

Summary

✅ 31 untouched benchmarks

@kaylendog kaylendog marked this pull request as ready for review August 15, 2025 14:45
@kaylendog kaylendog requested review from a team as code owners August 15, 2025 14:45
@kaylendog kaylendog requested review from bnjbvr, andybalaam and richvdh and removed request for a team and bnjbvr August 15, 2025 14:45
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, looks really good. Thanks for breaking it up for easy review.

A few comments, nits, and suggestsions.

Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good. I agree with Rich's comments, and I've added a couple of small comments and questions from me.

@kaylendog kaylendog force-pushed the kaylendog/msc3414/crypto branch from 17818dd to ec54819 Compare August 18, 2025 10:52
Copy link

codecov bot commented Aug 18, 2025

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.59%. Comparing base (4f0415d) to head (13ee4c8).
⚠️ Report is 12 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...trix-sdk-crypto/src/olm/group_sessions/outbound.rs 95.83% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5539   +/-   ##
=======================================
  Coverage   88.59%   88.59%           
=======================================
  Files         340      340           
  Lines       93836    93847   +11     
  Branches    93836    93847   +11     
=======================================
+ Hits        83133    83143   +10     
  Misses       6572     6572           
- Partials     4131     4132    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kaylendog kaylendog requested a review from richvdh August 18, 2025 11:29
@kaylendog kaylendog requested review from richvdh and andybalaam and removed request for andybalaam August 18, 2025 12:56
This commit also refactors out what would be common code between
::encrypt and ::encrypt_state to a helper ::encrypt_inner.

Signed-off-by: Skye Elliot <[email protected]>
Modifies `OlmMachine::decrypt_room_event_inner` to call a new method
`OlmMachine::verify_packed_state_key` which, if the event is a state
event, verifies that the original event's state key, when unpacked,
matches the state key and event type in the decrypted event content.

Introduces MegolmError::StateKeyVerificationFailed and
UnableToDecryptReason::StateKeyVerificationFailed which are thrown when
the verification fails.

Signed-off-by: Skye Elliot <[email protected]>
@kaylendog kaylendog force-pushed the kaylendog/msc3414/crypto branch from 9ae4a3a to 84ebbd9 Compare August 18, 2025 14:58
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than a few nits about comments in tests

@richvdh richvdh merged commit 7af1d3a into main Aug 19, 2025
52 checks passed
@richvdh richvdh deleted the kaylendog/msc3414/crypto branch August 19, 2025 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants